-
Notifications
You must be signed in to change notification settings - Fork 0
feat(frontend): cooperative-page #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces a comprehensive set of components and types for the Cooperative page in a React-based web application. The changes include creating new React components like Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
❌ Deploy Preview for kleros-website-v2 failed. Why did it fail? →
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (21)
frontend/src/queries/cooperative/hero.ts (1)
4-4
: Remove unnecessary empty lineConsider removing the extra empty line to maintain consistent spacing.
frontend/src/components/Dropdown/DropdownItemButton.tsx (2)
4-6
: Fix typo in style variable nameThe variable name contains a typo:
dropwdownItemBaseStyle
should bedropdownItemBaseStyle
.
12-16
: Consider adding prop validationThe interface looks good, but consider adding prop-types validation for runtime type checking, especially for required props.
frontend/src/components/Cooperative/ReportSection/index.tsx (2)
13-22
: Consider simplifying the report type selectionThe switch statement could be simplified using an object lookup pattern for better maintainability.
- const getReports = (reportType: ReportType) => { - switch (reportType) { - case "annual": - return reportsData.annualReports; - case "risk": - return reportsData.riskReports; - default: - return reportsData.treasuryReports; - } - }; + const reportTypeMap = { + annual: reportsData.annualReports, + risk: reportsData.riskReports, + treasury: reportsData.treasuryReports, + }; + const getReports = (reportType: ReportType) => + reportTypeMap[reportType] ?? reportTypeMap.treasury;
12-34
: Consider adding error boundary and prop validationThe component could benefit from:
- An error boundary to handle potential data issues
- Explicit prop spreading instead of using the spread operator to avoid prop pollution
- Prop-types validation for runtime type checking
frontend/src/components/Cooperative/MemberSection/LearnMore.tsx (2)
16-16
: Replace hardcoded z-index values with CSS variablesUsing hardcoded z-index values (
z-[1]
) makes it difficult to maintain stacking context. Consider using CSS variables for better maintainability.- <h2 className="text-primary-text text-xl mb-8 z-[1]">{title}</h2> + <h2 className="text-primary-text text-xl mb-8 z-content">{title}</h2>Also applies to: 21-21, 24-24
28-30
: Use background.name for Image alt textThe alt text is hardcoded. Consider using
background.name
from props for better accessibility.- alt="Learn more Image Background" + alt={background.name}frontend/src/components/Cooperative/MemberSection/index.tsx (3)
5-7
: Remove Hungarian notation from interface nameThe
I
prefix (Hungarian notation) is not recommended in modern TypeScript. Consider renaming to justMemberSectionProps
.-interface IMemberSection { +interface MemberSectionProps { memberData: CooperativePageMemberQueryType["cooperativePageMemberSection"]; }
16-16
: Avoid spreading props blindlySpreading props (
{...memberData.learnMoreSection}
) can pass unwanted props. Consider explicit prop passing for better type safety and maintainability.- <LearnMore {...memberData.learnMoreSection} /> + <LearnMore + title={memberData.learnMoreSection.title} + button={memberData.learnMoreSection.button} + background={memberData.learnMoreSection.background} + />
23-23
: Avoid className overrides with !importantThe
!gap-2
className uses Tailwind's important modifier. Consider adjusting the ExternalLink component's base styles instead.frontend/src/pages/community.tsx (1)
Line range hint
31-44
: Remove redundant property name in getStaticPropsThe
heroData: heroData
assignment is redundant due to object property shorthand in modern JavaScript.return { props: { navbarData, footerData, - heroData: heroData, + heroData, }, };frontend/src/queries/cooperative/report-section.ts (3)
34-49
: Extract common fields into a base typeThe report types (
annualReports
,riskReports
,treasuryReports
) share common fields. Consider extracting them into a base type.type BaseReport = { url: string; year: number; }; type MonthlyReport = BaseReport & { month: string; }; export type CooperativePageReportQueryType = { cooperativePageReportSection: { reports: Report[]; }; annualReports: BaseReport[]; riskReports: MonthlyReport[]; treasuryReports: MonthlyReport[]; };
3-15
: Add JSDoc comments for exported typesThe exported types lack documentation. Consider adding JSDoc comments to improve code maintainability.
/** Type of report that can be displayed */ export type ReportType = "annual" | "risk" | "treasury"; /** Structure of a report item with its metadata */ export type Report = { title: string; // ... rest of the type };
17-51
: Consider breaking down the GraphQL queryThe query could be split into smaller fragments for better reusability and maintainability.
const reportFragment = gql` fragment ReportFields on Report { title subtitle reportType yearDropdownLabel monthDropdownLabel downloadButtonText icon { name url } } `; export const cooperativePageReportQuery = gql` { cooperativePageReportSection { reports { ...ReportFields } } # ... rest of the query } ${reportFragment} `;frontend/src/components/Cooperative/ReportSection/ReportCard.tsx (1)
9-17
: Consider adding type constraints for the month field.The
month
field inReports
type could benefit from being more specific about the expected format (e.g., full month name, abbreviated month, or numeric).export type Reports = { url: string; - month?: string; + month?: 'January' | 'February' | 'March' | 'April' | 'May' | 'June' | + 'July' | 'August' | 'September' | 'October' | 'November' | 'December'; year: number; }[];frontend/src/components/Cooperative/hero.tsx (2)
23-37
: Enhance button accessibility for external links.When opening links in new tabs, it's important to inform screen reader users.
{buttons.map((button) => ( <Link key={button.text} href={button.link.url} target="_blank" rel="noopener noreferrer" + aria-label={`${button.text} (opens in new tab)`} > <Button variant="secondary" className=" hover:!bg-primary-blue hover:!border-primary-blue hover:!text-background-2" > <span>{button.text}</span> </Button> </Link> ))}
45-50
: Optimize hero background image loading.Consider adding priority and quality props to optimize the hero image loading performance.
<Image src={background.url} alt="Hero Image Background" fill + priority + quality={90} className="absolute top-0 left-0 h-full z-[-1] object-cover" />frontend/src/components/Dropdown/index.tsx (1)
36-49
: Improve event handler type safety.Replace
any
with proper MouseEvent type for better type safety.useEffect(() => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const handleClickOutside = (event: any) => { + const handleClickOutside = (event: MouseEvent) => { if (dropdownRef.current && !dropdownRef.current.contains(event.target as Node)) { setIsOpen(false); } }; document.addEventListener("mousedown", handleClickOutside); return () => { document.removeEventListener("mousedown", handleClickOutside); }; }, []);frontend/src/components/Cooperative/ReportSection/DropdownContainer.tsx (3)
18-19
: Consider initializing state with meaningful default valuesThe undefined initial states could lead to uncontrolled components initially. Consider:
- Initializing with the most recent year from reports
- Setting the first available month for that year
- const [year, setYear] = useState<number>(); - const [month, setMonth] = useState<string>(); + const [year, setYear] = useState<number>(() => + Math.max(...Array.from(reports.map(r => r.year))) + ); + const [month, setMonth] = useState<string>(() => { + const reportsForYear = reports.filter(r => r.year === year); + return reportsForYear[0]?.month || ''; + });
52-61
: Consider moving monthOrder near its usageThe month sorting logic relies on monthOrder array defined at the bottom of the file. Consider:
- Moving monthOrder declaration above its usage
- Making it a constant using
as const
for better type safety+ const monthOrder = [ + "January", "February", "March", "April", "May", "June", + "July", "August", "September", "October", "November", "December" + ] as const; + const months = useMemo( () => Array.from(new Set(monthsSet)) .sort((a, b) => monthOrder.indexOf(a) - monthOrder.indexOf(b)) .map((month) => ({ key: month, value: month, })), [monthsSet] );
101-115
: Consider using named exportsFor better maintainability and import/export tracking:
- export default DropdownContainer; + export { DropdownContainer };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/src/assets/svgs/icons/down-arrow-blue.svg
is excluded by!**/*.svg
📒 Files selected for processing (13)
frontend/src/components/Cooperative/MemberSection/LearnMore.tsx
(1 hunks)frontend/src/components/Cooperative/MemberSection/index.tsx
(1 hunks)frontend/src/components/Cooperative/ReportSection/DropdownContainer.tsx
(1 hunks)frontend/src/components/Cooperative/ReportSection/ReportCard.tsx
(1 hunks)frontend/src/components/Cooperative/ReportSection/index.tsx
(1 hunks)frontend/src/components/Cooperative/hero.tsx
(1 hunks)frontend/src/components/Dropdown/DropdownItemButton.tsx
(1 hunks)frontend/src/components/Dropdown/index.tsx
(1 hunks)frontend/src/pages/community.tsx
(2 hunks)frontend/src/pages/cooperative.tsx
(1 hunks)frontend/src/queries/cooperative/hero.ts
(1 hunks)frontend/src/queries/cooperative/member-section.ts
(1 hunks)frontend/src/queries/cooperative/report-section.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - kleros-website-v2
- GitHub Check: Header rules - kleros-website-v2
- GitHub Check: Pages changed - kleros-website-v2
🔇 Additional comments (9)
frontend/src/queries/cooperative/hero.ts (1)
5-27
: LGTM! Well-structured GraphQL query with matching TypeScript typesThe type definitions accurately match the GraphQL query structure, and the reuse of the
ArrowLink
type promotes consistency across the application.Also applies to: 29-44
frontend/src/components/Dropdown/DropdownItemButton.tsx (1)
17-32
: LGTM! Clean implementation of dropdown item buttonThe component is well-structured with proper type safety and clean conditional styling using clsx.
frontend/src/queries/cooperative/member-section.ts (1)
5-49
: LGTM! Well-organized types and query structureThe implementation shows good type safety with proper reuse of the
ArrowLink
type and clean hierarchy between types and GraphQL query structure.frontend/src/components/Cooperative/MemberSection/LearnMore.tsx (1)
15-15
: Add explicit dimensions to the parent containerThe Image component uses
fill
prop but the parent container lacks explicit dimensions, which may cause layout issues. Add height/width to the parent div.- <div className="relative w-full flex flex-col items-center justify-center mt-16 p-8 "> + <div className="relative w-full h-[300px] flex flex-col items-center justify-center mt-16 p-8 ">Also applies to: 27-32
frontend/src/pages/community.tsx (1)
45-45
: LGTM! Component renaming improves clarityThe renaming from
ForBuilders
toCommunity
better reflects the component's purpose.frontend/src/components/Cooperative/ReportSection/ReportCard.tsx (1)
53-55
: LGTM! Well-implemented responsive image.The Image component is properly configured with fill property and appropriate parent container dimensions.
frontend/src/components/Cooperative/ReportSection/DropdownContainer.tsx (3)
1-10
: Well-structured type definitions and imports!The interface is properly defined using TypeScript's Pick utility type, ensuring type safety for the component props.
73-98
: Well-structured and accessible render logic!The component demonstrates good practices:
- Responsive layout using Tailwind CSS
- Proper accessibility with labels
- Clean conditional rendering
1-115
: Verify edge cases and performancePlease ensure thorough testing of:
- Behavior when reports array is empty
- Performance with large numbers of reports
- Correct handling of invalid year/month combinations
✅ Verification successful
Implementation is robust and well-designed
The component already handles the mentioned concerns effectively:
- Empty reports are handled safely through Set operations
- Performance is optimized with useMemo for large datasets
- Type safety and controlled inputs prevent invalid combinations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for test coverage of this component fd -e test.tsx -e spec.tsx -e test.ts -e spec.ts . | xargs rg "DropdownContainer"Length of output: 629
Script:
#!/bin/bash # Search for ReportCard tests and implementation fd "ReportCard" . -e test.tsx -e spec.tsx -e test.ts -e spec.ts | xargs cat # Examine error handling in ReportCard rg -A 10 "reports.*=" frontend/src/components/Cooperative/ReportSection/ReportCard.tsxLength of output: 164
Script:
#!/bin/bash # Check for error boundaries and type definitions rg -l "ErrorBoundary" frontend/src/components/Cooperative/ReportSection/ # Examine Report type definition cat frontend/src/queries/cooperative/report-section.ts # Check component's error handling rg -A 5 "catch|throw|error" frontend/src/components/Cooperative/ReportSection/DropdownContainer.tsxLength of output: 1369
frontend/src/components/Cooperative/ReportSection/ReportCard.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/Cooperative/ReportSection/DropdownContainer.tsx
Outdated
Show resolved
Hide resolved
Refactor(frontend)/cooperative page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Summary by CodeRabbit
New Features
LearnMore
component for additional information display.DropdownContainer
for selecting report parameters.ReportCard
component to showcase individual reports.Dropdown
component for improved item selection.Hero
component for the cooperative page layout.Improvements
Changes
ForBuilders
page toCommunity
.Button
component to include a new tertiary style variant.